Skip to content

feat(gateway): WinRM operations for PAM dependency detection and rotation (PAM-350)#321

Open
bernie-g wants to merge 4 commits into
mainfrom
bernie/pam-350-implement-support-for-domain-account-dependency-detection
Open

feat(gateway): WinRM operations for PAM dependency detection and rotation (PAM-350)#321
bernie-g wants to merge 4 commits into
mainfrom
bernie/pam-350-implement-support-for-domain-account-dependency-detection

Conversation

@bernie-g

@bernie-g bernie-g commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Companion PR (backend + frontend): Infisical/infisical#7355

Adds the gateway's WinRM operations for finding Windows account dependencies and updating their passwords during rotation.

…ion (PAM-350)

Add enumerate-accounts, enumerate-dependencies, rotate-credential, and
sync-dependency WinRM operations so the control plane names a vetted operation
and passes parameters instead of sending raw PowerShell. Credential changes use
pure-PowerShell cmdlets (CIM Win32_Service.Change, Set-ScheduledTask,
Set-ADAccountPassword) so a password never crosses a native-exe argument line.
@linear

linear Bot commented Jul 21, 2026

Copy link
Copy Markdown

PAM-350

@infisical-review-police

Copy link
Copy Markdown

💬 Discussion in Slack: #pr-review-cli-321-feat-gateway-winrm-operations-for-pam-dependency-detection-and

Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel.

bernie-g added 3 commits July 20, 2026 21:37
- Change service credentials via CIM Win32_Service.Change and scheduled tasks via
  Set-ScheduledTask (pure PowerShell) so a password with quotes isn't mangled by
  native-exe argument quoting.
- Only restart a service/app-pool that was already running.
- Include InteractiveOrPassword scheduled tasks; fail the enumeration hard on a
  service/task error so partial results can't prune real dependency rows.
- Add a validate-credential op so the admin rotator can verify a local account's
  password on the box (ValidateCredentials) without logging in as the account.
- Only restart services/app-pools that were already running (done earlier), and
  now: harden local-account enumeration with ErrorActionPreference=Stop, include
  InteractiveOrPassword scheduled tasks, and gate IIS enumeration on the module
  being present so a real IIS error hard-fails instead of being swallowed.
- winrm/pam.go: resolve a scheduled task's run-as principal through the host's
  LSA (SID round-trip) so a member server's local account is not mis-anchored to
  a same-named domain account
- winrm/winrm.go: decode PowerShell CLIXML stderr to plain text so a WinRM
  command failure surfaces the real error instead of the raw XML envelope
switch kind {
case "local":
script = fmt.Sprintf(
`$ErrorActionPreference='Stop'; Set-LocalUser -Name '%s' -Password (ConvertTo-SecureString '%s' -AsPlainText -Force)`,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium: Plaintext credentials in PowerShell logs

newPassword is embedded directly in the script text before ConvertTo-SecureString, so anyone able to read PowerShell script-block or process-command auditing can recover it. The same pattern occurs in domain rotation, credential validation, and every dependency-sync branch; pass the secret separately from the script text, such as through the WinRM command's input stream, and suppress or redact command error output.

@veria-ai

veria-ai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

PR overview

This PR adds WinRM-based PAM operations in the gateway for detecting account dependencies and performing credential validation, password rotation, and dependency synchronization on Windows targets. The touched code implements PowerShell-driven local and domain account workflows under the gateway WinRM package.

One security issue remains open: newly rotated or validated passwords are interpolated directly into PowerShell script text, which can expose them through script-block logging, command-line auditing, or captured error output. This creates a concrete credential disclosure risk for anyone with access to those logs or process records. No issues have been addressed yet, so the PR still needs changes to pass secrets out-of-band and avoid leaking them in logged command text.

Open issues (1)

Fixed/addressed: 0 · PR risk: 7/10

@greptile-apps

greptile-apps Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds WinRM operations for Windows PAM dependency discovery and credential rotation. The main changes are:

  • Account, service, scheduled-task, and IIS app-pool enumeration.
  • Local and domain account password rotation.
  • Dependency password synchronization and local credential validation.
  • New gateway routes for the WinRM operations.
  • Readable decoding of PowerShell CLIXML errors.

Confidence Score: 4/5

The IIS app-pool synchronization path needs fixes before merging.

  • Wildcard characters in pool names can redirect credential writes.
  • A failed password write can leave the pool with mismatched credentials.
  • The other changed handlers and build surface show no blocking issue.

packages/gateway-v2/winrm/pam.go

Security Review

IIS provider wildcard expansion can apply credentials to an unintended app pool when a dependency name contains wildcard characters.

Important Files Changed

Filename Overview
packages/gateway-v2/winrm/pam.go Adds privileged discovery and credential operations; IIS synchronization can select the wrong pool or leave a partial credential update.
packages/gateway-v2/winrm/winrm.go Adds CLIXML error cleanup using standard-library regular expressions and numeric parsing.
packages/gateway-v2/winrm_handler.go Registers and implements five new WinRM request handlers.

Reviews (1): Last reviewed commit: "gateway: qualify scheduled-task principa..." | Re-trigger Greptile

Comment on lines +207 to +210
script = fmt.Sprintf(
`$ErrorActionPreference='Stop'; Import-Module WebAdministration; `+
`Set-ItemProperty 'IIS:\AppPools\%s' -Name processModel.userName -Value '%s'; `+
`Set-ItemProperty 'IIS:\AppPools\%s' -Name processModel.password -Value '%s'; `+

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security App Pool Path Expands Wildcards

When an app-pool name contains PowerShell wildcard characters such as [ or ], the positional IIS provider path can resolve as a pattern instead of a literal name. Credential synchronization can then update the wrong pool, multiple pools, or no pool while the later -Name lookup targets the literal dependency.

Comment on lines +207 to +210
script = fmt.Sprintf(
`$ErrorActionPreference='Stop'; Import-Module WebAdministration; `+
`Set-ItemProperty 'IIS:\AppPools\%s' -Name processModel.userName -Value '%s'; `+
`Set-ItemProperty 'IIS:\AppPools\%s' -Name processModel.password -Value '%s'; `+

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Credential Update Can Partially Commit

The username and password are written in separate commands. If the username write succeeds and the password write fails, this function returns an error but leaves the pool using the new identity with the previous password, causing authentication failures on its next start or recycle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant